-
Notifications
You must be signed in to change notification settings - Fork 25
Switch to Google-style docstrings with Markdown and remove Sphinx/RST generation #592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jonathan343
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Antonio, this is a great start!
This PR can really be broken down into two main components:
- Formatting docstrings to comply with the requirements of MkDocs
- Generating the MkDocs specific files and static
.md
The first 100% needs to happen at codegen time. However, the more I look at this, the more I feel like doing 2 during codegen time and committing all the static files will really bloat our SDK repo when we're working with 400+ clients. This open PR is adding 4000+ lines for one client. The more scalable approach here might be to do all this generation on the fly when needed like we do with botocore.
I don't see anything in the generated files in https://github.com/awslabs/aws-sdk-python/pull/24/files that would make it difficult to do the approach mentioned above. The generated clients themselves have all the information we need.
...aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsMkDocsFileGenerator.java
Outdated
Show resolved
Hide resolved
jonathan343
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good so far, thanks Antonio! Just had a few comments/questions.
...n/core/src/test/java/software/amazon/smithy/python/codegen/writer/MarkdownConverterTest.java
Show resolved
Hide resolved
| :param plugins: A list of callables that modify the configuration dynamically. These | ||
| can be used to set defaults, for example.""", rstDocs); | ||
| }); | ||
| .orElse("Client for " + service.getId().getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this change from .orElse("Client for " + serviceSymbol.getName());?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a minor improvement to the default client description in cases where a Smithy service does not have a service-level documentation trait.
serviceSymbol.getName() returns the client class name so the default description looks like "Client for BedrockRuntimeClient" which seems wrong to me.
This switches the default docstring to use the service id. For Bedrock Runtime, this looks like "Client for AmazonBedrockFrontendService".
I did consider using the AWS sdk id but that would exclude non-AWS services. I also considered using the smithy.api#title trait for the service but its not required so a service without it fails to generate.
This was only a minor improvement I noticed and don't feel too strong about it.
codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/SetupGenerator.java
Outdated
Show resolved
Hide resolved
codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/MarkdownConverter.java
Outdated
Show resolved
Hide resolved
jonathan343
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This PR updates the generated docstring format to use the Google style with Markdown descriptions for all services (AWS and non-AWS).
This PR also removes the Sphinx-based reStructuredText (RST) documentation generation system introduced in #418. All documentation stubs will now be generated at build time in the
awslabs/aws-sdk-pythonrepo. This will prevent static doc stubs from bloating the repo when 400+ services are supported.Key changes:
Client,Structure,Config,Enum,Union) to generate Google style docstrings with Markdown descriptions using the newMarkdownConverterclassAwsRstDocFileGeneratorplugin that generates.rstfiles for Sphinx doc genNote
The new docstring format in AWS clients enables us to generate documentation using Material for MkDocs. We will generate MkDocs stubs in awslabs/aws-sdk-python that work with the
mkdocstringstool. This will automatically create documentation from docstrings for all clients.Testing
Important
For local testing, please install pandoc v3.8.2 before running code generator.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.